Skip to content

Conversation

@DTCurrie
Copy link
Member

@DTCurrie DTCurrie commented Nov 7, 2025

Before we add more unit tests to the repository, I am reorganizing things to help us have consistent and maintainable tests, as well as some utilities to make writing tests easier. I know this is a large PR, but it keeps all of current tests and just updates them to follow recommended best practices for Vitest/TypeScript projects.

All tests follow the Arrange-Act-Assert pattern with explicit comments (// Arrange, // Act, // Assert) to clearly delineate test phases. We prioritize per-test mock setup using helper functions (e.g., setupDialWebRTCMocks()) over beforeEach() hooks to make tests self-contained. Parameterized tests using it.each() reduce duplication, and all mocks are cleaned up in afterEach() with vi.restoreAllMocks(). Critically, we've eliminated all test logic anti-patterns—no loops, conditionals, or mutable state variables—instead using Vitest's declarative chaining API (.mockReturnValueOnce(), .mockImplementationOnce()) for predictable test behavior.

We've adopted a three-directory structure: __tests__/ contains test files (*.spec.ts), __fixtures__/ contains test data and configuration objects as individual named constants (e.g., withAccessToken, testCredential), and __mocks__/ contains reusable mock factory functions that return fresh instances (e.g., createMockPeerConnection()). The key distinction is that fixtures are data (configurations, constants, sample values) while mocks are behavior (factory functions that create test doubles with methods). We use stubs when we only need canned responses and verify the resulting state, and mocks when we need to verify specific interactions occurred. Fake timers (vi.useFakeTimers()) ensure deterministic testing of time-dependent code. This separation provides clear boundaries while enabling reusability and preventing state pollution between tests. It also keeps domain directories clean and focused on "business logic" and keeps domain-related test logic in clear and consistent locations.

@DTCurrie DTCurrie self-assigned this Nov 7, 2025
@DTCurrie DTCurrie requested a review from a team as a code owner November 7, 2025 21:21
@DTCurrie DTCurrie requested review from lia-viam and njooma November 7, 2025 21:21
Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks reasonable to me, I only just skimmed this but it appears to be basically just code moves. if there's anything that needs a particularly close eye on it please point me towards it!

@DTCurrie
Copy link
Member Author

looks reasonable to me, I only just skimmed this but it appears to be basically just code moves. if there's anything that needs a particularly close eye on it please point me towards it!

That's it, more or less. Just some reorganization to keep things clean and consistent. It's also how we manage tests in other frontend repositories, so this will maintain consistency across the organization.

@DTCurrie DTCurrie merged commit 6eb8027 into main Nov 11, 2025
3 checks passed
@DTCurrie DTCurrie deleted the test-reorg-and-cleanup branch November 11, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants